-
Notifications
You must be signed in to change notification settings - Fork 6
propose a new tx to cancel the old evm tx that cannot be sent #122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements a mechanism to cancel stuck EVM transactions that fail validation on the Gnosis Safe contract but cannot be re-broadcast. When a transaction remains stuck on-chain for more than 30 minutes, it can be marked as stuck and a new cancellation transaction with the same nonce can be proposed to replace it.
Key changes:
- Adds
FlagProposeCancelTransactionto propose cancellation transactions that reuse the stuck transaction's nonce - Introduces a
stuckboolean column in thetransactionstable to track stuck transactions - Modifies transaction broadcasting to detect and mark stuck transactions, then build and send replacement transactions directly
- Removes the deprecated
ActionEthereumSafeRefundTransactionflow in favor of the new cancellation mechanism
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| common/request.go | Renumbers transaction flag constants to add FlagProposeCancelTransaction (value 2) and shifts inheritance flags to 3-4 |
| apps/ethereum/account.go | Adds FetchSafeNonce function to retrieve the nonce of a Safe contract at a specific block height |
| apps/ethereum/rpc.go | Updates RPCGetBlockByHeight to return full block details including timestamp |
| apps/ethereum/transaction.go | Adds BuildTransaction method to construct signed transactions without sending them, and updates ValidTransaction to return only error |
| observer/schema.sql | Adds stuck boolean column to transactions table |
| observer/migrate.go | Adds migration to add stuck column and update specific stuck transaction |
| observer/store.go | Updates Transaction model with Stuck field and adds MarkTransactionStuck method; updates queries to handle stuck transactions |
| observer/accountant.go | Refactors ethereumBroadcastTransactionAndWriteDeposit to detect stuck transactions, build replacement transactions, and verify broadcast success |
| observer/ethereum.go | Updates network info loop to use new RPCGetBlockByHeight method |
| observer/node.go | Adds getStuckTxHash helper to retrieve the stuck transaction hash from a cancellation transaction; removes obsolete migration call |
| observer/http.go | Updates transaction endpoint to show "stuck" state for stuck transactions |
| keeper/node.go | Adds getTransactionFlagAndExtra helper and migration call |
| keeper/group.go | Removes handling for deprecated ActionEthereumSafeRefundTransaction |
| keeper/ethereum.go | Implements cancellation transaction proposal and approval logic; validates stuck transactions and creates replacement transactions with same nonce |
| keeper/ethereum_test.go | Adds test for cancellation transaction flow; removes test for deprecated refund action |
| keeper/bitcoin.go | Updates to use new signature method signature and refactored inheritance lock helper |
| keeper/store/transaction.go | Adds CancelPrevious field to Transaction struct; removes deprecated FailTransactionWithRequest method |
| keeper/store/signature.go | Updates FinishTransactionSignaturesWithRequest to skip nonce increment for cancellation transactions |
| keeper/store/request.go | Refactors ReadRequest to use helper method for reuse |
| keeper/store/migrate.go | Adds migration to fix specific stuck transaction state and balances |
| signer/ethereum_test.go | Adds tests for FetchSafeNonce at various block heights |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
observer/accountant.go
Outdated
| if err != nil { | ||
| panic(err) | ||
| } | ||
| time.Sleep(time.Second * 30) |
Copilot
AI
Dec 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code sleeps for 30 seconds after sending the transaction before checking if it's in the mempool. This hardcoded delay may be too long for fast networks or too short for congested networks. Consider making this configurable or using an exponential backoff strategy.
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| defer conn.Close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This connection should not be returned, and also already closed.
apps/ethereum/transaction.go
Outdated
| return conn, t, nil | ||
| } | ||
|
|
||
| func (tx *SafeTransaction) ExecTransaction(ctx context.Context, rpc, key string) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function could be removed, and add SendAndWaitMined?
No description provided.